Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d87cabc) 98.21% compared to head (1bd23da) 98.30%.

@@            Coverage Diff             @@
##           master    #1951      +/-   ##
==========================================
+ Coverage   98.21%   98.30%   +0.09%     
==========================================
  Files         800      833      +33     
  Lines       12501    13176     +675     
==========================================
+ Hits        12278    12953     +675     
  Misses        223      223              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Job #2314: Bundle Size — 9.82MiB (+1.9%).

1bd23da(current) vs d87cabc master#2297(baseline)

Bundle metrics   5 changes 2 regressions

	Current Job #2314	Baseline Job #2297
Initial JS	5.34MiB(+2.01%)	5.23MiB
Initial CSS	303.16KiB(+0.2%)	302.56KiB
Cache Invalidation	58%	0%
Chunks	51	51
Assets	164(+1.86%)	161
Modules	1454(+1.25%)	1436
Duplicate Modules	20	20
Duplicate Code	0.8%	0.8%
Packages	124	124
Duplicate Packages	3	3

Bundle size by type   4 changes 4 regressions

	Current Job #2314	Baseline Job #2297
JS	7.03MiB (+1.53%)	6.92MiB
IMG	2.36MiB (+3.32%)	2.28MiB
CSS	319.67KiB (+0.19%)	319.07KiB
Fonts	93.55KiB	93.55KiB
Other	16.82KiB (+1.92%)	16.5KiB
HTML	13.58KiB	13.58KiB

View job #2314 report View Terdious:netatmo-integration branch activity View project dashboard

Hi @Pierre-Gilles
Couldn't it be beneficial to do a first review for this part which is new (I want to talk in particular about the OAut2 connection).
This would also prevent the review from being too long.
And also for me not to go work on something else if this start is not valid.

Thanks in advance.

Hi @Pierre-Gilles

When I first open the integration, here is what I see:

Screenshot 2024-01-11 at 09 54 11

There is an error message immediately (I didn't do anything), which is a bit weird for the user.

So for now I can't even reproduce your first connection error... Nobody has obviously had the case on the 2 testers and I have just started again with a fresh installation on wsl, I don't have this problem ..., my state first connection :

Also test via the docker image, my state first connection :

Could you see on your side to guide me?

not very clean, we don't put error message here, it's a menu

Ok for the message part in the left banner, it allowed us to have a common visual and removed the repetition aspect and it seems to me that the 2 testers found it clean on the contrary. I withdraw.

Link to the documentation is missing category:
Screenshot 2024-01-11 at 09 54 30

Corrected, my bad !

Buttons a bit weird:

Screenshot 2024-01-11 at 09 55 08

Yes for the button, I wanted things to be clearly expressed. This is not just a matter of saving the modified data, but of having to request new access and therefore go through the Netatmo login page each time you save. I make a proposal in this sense to put a text above. Corrected

There is a major issue with the way the NetatmoHandler works, it seems you are passing the object (itself) to every single function. It's an anti-pattern, you have access to this everywhere.

For the "netatmoHandler" passed around... I guess it's a poor understanding/configuration of my vscode. The “this.” » is declared in eslint error everywhere in my vscode interface, so I passed the 'netatmoHandler' for this reason only. My bad, corrected

Many files of this class contains several functions. Try to keep 1 file = 1 function when you are building functions of a class (Talking about NetatmoHandler)

I understand. Corrected

On the UI, it looks good to me now! Can you update your documentation PR with new screenshots?

I do this before the end of the evening.